Set override_path = true in hosts.toml#10765
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10765 +/- ##
=======================================
Coverage 72.30% 72.31%
=======================================
Files 608 608
Lines 39388 39402 +14
=======================================
+ Hits 28481 28493 +12
- Misses 9173 9176 +3
+ Partials 1734 1733 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… v2 migration URLs are used as-is) to hosts.toml format (where containerd auto-appends /v2/ to the host path). Since ToAPIEndpoints() already prepends /v2/ before the namespace, containerd produced a broken double /v2/ in requests: /v2/<namespace>/v2/<repo>/manifests/<tag> → 404. Fix: Always set override_path = true in hosts.toml entries so containerd uses the host path as-is without auto-appending /v2/. Also update ToAPIEndpoint to always produce a /v2 path prefix (even for pathless URLs) ensuring all entries have a well-formed OCI API root. Signed-off-by: Rahul Ganesh <rahulgab@amazon.com>
bcd55c6 to
bd4a7c1
Compare
|
/cherrypick release-0.25 |
|
@rahulbabu95: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
override_path = true to namespaced hosts.toml entries in hosts.tomloverride_path = true in hosts.toml
|
/cherrypick release-0.24 |
|
@rahulbabu95: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: snarkychef The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue #, if available:
#10212 introduced a regression while migrating the registry mirror config from config.toml (registry.mirrors endpoint format) to
hosts.tomlformat. When oci namespaces are configured, ToAPIEndpoints() prepends /v2/ to the namespace path in the host URL. In the old
config.tomlformat this was used as-is. Inhosts.tomlformat, containerd's parser auto-appends an additional /v2/ to the host path, causing a double /v2/ in requests:/v2/< namespace>/v2/<repo>/manifests/<tag>resulting in 404.Description of changes:
Add
override_path = trueto namespacedhosts.tomlentries. This instructs containerd to use the host path as is without auto-appending /v2/. For base mirror entry (no path) we always append /v2/ for direct repo references so thatoverride_pathcan be set unconditionally safely.Testing (if applicable):
Pre-fix (setup a test ns rahul-tst in our CI registry and validate the image pull URIs from the node):
Hosts.toml:
Image pull logs:
May 08 23:02:02 containerd[1404]: level=debug msg="fetch response received" response.status="404 Not Found" url="https://10.80.148.51:443/v2/rahul-test/v2/eks/cilium/cilium/manifests/v1.19.1-0?ns=public.ecr.aws"Post-fix:
Hosts.toml:
response.status="200 OK" content-type=application/vnd.oci.image.manifest.v1+json url="https://10.80.148.51:443/v2/rahul-test/eks/cilium/cilium/manifests/v1.19.1-0?ns=public.ecr.aws"Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.